Go: Promote non-httponly cookie query, and add insecure cookie query #20762

joefarebrother · 2025-11-05T11:01:45Z

Promotes go/cookie-httponly-not-set from experimental, and adds go/cookie-secure-not-set query.

go/ql/lib/semmle/go/concepts/HTTP.qll

go/ql/src/Security/CWE-1004/CookieWithoutHttpOnly.ql

go/ql/src/Security/CWE-614/CookieWithoutSecure.ql

github-actions · 2025-11-10T09:32:18Z

QHelp previews:

go/ql/src/Security/CWE-1004/CookieWithoutHttpOnly.qhelp

Cookie 'HttpOnly' attribute is not set to true

Cookies without the HttpOnly flag set are accessible to client-side scripts such as JavaScript running in the same origin. In case of a Cross-Site Scripting (XSS) vulnerability, the cookie can be stolen by a malicious script. If a sensitive cookie does not need to be accessed directly by client-side JS, the HttpOnly flag should be set.

Recommendation

Set the HttpOnly flag to true for authentication cookies to ensure they are not accessible to client-side scripts.

Example

In the following example, in the case marked BAD, the HttpOnly flag is not set, so the default value of false is used. In the case marked GOOD, the HttpOnly flag is set to true.

package main

import (
	"net/http"
)

func handlerBad(w http.ResponseWriter, r *http.Request) {
	c := http.Cookie{
		Name:  "session",
		Value: "secret",
	}
	http.SetCookie(w, &c) // BAD: The HttpOnly flag is set to false by default.
}

func handlerGood(w http.ResponseWriter, r *http.Request) {
	c := http.Cookie{
		Name:     "session",
		Value:    "secret",
		HttpOnly: true,
	}
	http.SetCookie(w, &c) // GOOD: The HttpOnly flag is set to true.
}

References

MDN: Set-Cookie Header.
PortSwigger: Cookie without HttpOnly flag set
Common Weakness Enumeration: CWE-1004.

go/ql/src/Security/CWE-614/CookieWithoutSecure.qhelp

Cookie 'Secure' attribute is not set to true

Cookies without the Secure flag set may be transmitted using HTTP instead of HTTPS. This leaves them vulnerable to being read by a third party attacker. If a sensitive cookie such as a session key is intercepted this way, it would allow the attacker to perform actions on a user's behalf.

Recommendation

Set the Secure flag to true to ensure cookies are only transmitted over secure HTTPS connections.

Example

In the following example, in the case marked BAD, the Secure flag is set to false by default. In the case marked GOOD, the Secure flag is set to true.

package main

import (
	"net/http"
)

func handlerBad(w http.ResponseWriter, r *http.Request) {
	c := http.Cookie{
		Name:  "session",
		Value: "secret",
	}
	http.SetCookie(w, &c) // BAD: The Secure flag is set to false by default.
}

func handlerGood(w http.ResponseWriter, r *http.Request) {
	c := http.Cookie{
		Name:   "session",
		Value:  "secret",
		Secure: true,
	}
	http.SetCookie(w, &c) // GOOD: The Secure flag is set to true.
}

References

MDN: Set-Cookie Header.
Detectify: Cookie lack Secure flag.
PortSwigger: TLS cookie without secure flag set.
Common Weakness Enumeration: CWE-614.

Copilot

Pull Request Overview

This PR promotes an experimental query for detecting cookies without the HttpOnly flag and introduces a new query for detecting cookies without the Secure flag. The changes include:

Migration of the CookieWithoutHttpOnly query from experimental to the main security query pack
Introduction of a new CookieWithoutSecure query
New shared library code (SecureCookies.qll) for modeling HTTP cookie security attributes
Updates to framework models for net/http and gin-gonic/gin

Reviewed Changes

Copilot reviewed 31 out of 42 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
go/ql/src/Security/CWE-1004/CookieWithoutHttpOnly.ql	New query for detecting cookies without HttpOnly flag
go/ql/src/Security/CWE-614/CookieWithoutSecure.ql	New query for detecting cookies without Secure flag
go/ql/lib/semmle/go/security/SecureCookies.qll	Shared library for cookie security analysis
go/ql/lib/semmle/go/concepts/HTTP.qll	Added `CookieWrite` and `CookieOptions` concepts
go/ql/lib/semmle/go/frameworks/NetHttp.qll	Added cookie write models for net/http
go/ql/lib/semmle/go/frameworks/Gin.qll	New framework models for gin-gonic/gin
go/ql/src/change-notes/2025-11-10-inseucre-cookie.md	Change notes documenting the updates
go/ql/src/experimental/CWE-1004/*	Removed experimental query files
go/ql/test/query-tests/Security/CWE-614/*	Test files for CookieWithoutSecure
go/ql/test/query-tests/Security/CWE-1004/*	Test files for CookieWithoutHttpOnly

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

go/ql/src/change-notes/2025-11-10-insecure-cookie.md

go/ql/src/Security/CWE-614/CookieWithoutSecure.ql

go/ql/lib/semmle/go/concepts/HTTP.qll

owen-mc

I love the new concepts. I think they make the structure quite clean, and allow us to add other libraries easily in future.

I've only done a partial review so far. It will be easier to continue once these points have been addressed.

I note that you've removed modelling and tests for github.com/gorilla/sessions. Was that deliberate? It seems like a popular enough library, and it's only a bit of extra modelling and some tests that have already been written.

go/ql/lib/semmle/go/concepts/HTTP.qll

go/ql/lib/semmle/go/frameworks/stdlib/NetHttp.qll

go/ql/lib/semmle/go/concepts/HTTP.qll

go/ql/lib/semmle/go/security/SecureCookies.qll

go/ql/src/Security/CWE-614/CookieWithoutSecure.ql

go/ql/lib/semmle/go/security/SecureCookies.qll

joefarebrother · 2025-11-17T20:32:22Z

Addressed comments
I can re-add the gorilla/sessions models; they were initially ommited due to a slight complication in which there isn't a sensitive name expression to alert for as the source; however it can be handled by alerting for the cookie constructor with a name of something like "Gorilla session".

owen-mc

Thanks for applying all those changes. On second thought, maybe it's too much work to add modeling for Gorilla. You can leave it for now. But please remove the stubs for it, and mentions of it from the go.mod and modules.txt files.

My only slight worry is that performance might be bad in certain situations. Let me explain. For the insecure cookie query, to get the best performance I think you would have two data flow configs: one from getCookieOutput() of CookieOptionsWrite for getSecure exists to CookieWrite, and one from any boolean typed node with getBooleanValue() = false to the getSecure() of a CookieOptionsWrite (for extra performance you would also require that the getCookieOutput() of it is a source a path for the first data flow config. By using just one config you've made is simpler, but you could end up with lots of sources along the same path, and hence lots of paths. Maybe that's okay performance-wise. But the DCA run seems fine, so no need to change anything - we can always look into it in future if needed.

owen-mc · 2025-11-18T12:07:54Z

go/ql/lib/semmle/go/concepts/HTTP.qll

+  /** Provides a class for modeling the new APIs for writes to options of an HTTP cookie. */
+  module CookieOptionWrite {
+    /**
+     * A write to an HTTP cookie object.


Suggested change

* A write to an HTTP cookie object.

* A write to an option of an HTTP cookie object.

owen-mc · 2025-11-18T12:08:35Z

go/ql/lib/semmle/go/concepts/HTTP.qll

+     * A write to an HTTP cookie object.
+     *
+     * Extend this class to model new APIs. If you want to refine existing API models,
+     * extend `HTTP::CookieOptions` instead.


Suggested change

* extend `HTTP::CookieOptions` instead.

* extend `HTTP::CookieOptionWrite` instead.

owen-mc · 2025-11-18T12:08:56Z

go/ql/lib/semmle/go/concepts/HTTP.qll

+  }
+
+  /**
+   * A write to an HTTP cookie object.


Suggested change

* A write to an HTTP cookie object.

* A write to an option of an HTTP cookie object.

owen-mc · 2025-11-18T12:09:26Z

go/ql/lib/semmle/go/concepts/HTTP.qll

+   * A write to an HTTP cookie object.
+   *
+   * Extend this class to refine existing API models. If you want to model new APIs,
+   * extend `HTTP::CookieOptions::Range` instead.


Suggested change

* extend `HTTP::CookieOptions::Range` instead.

* extend `HTTP::CookieOptionWrite::Range` instead.

owen-mc · 2025-11-18T14:48:35Z

go/ql/src/Security/CWE-1004/CookieWithoutHttpOnly.ql

+  Http::CookieWrite cw, string name, SensitiveCookieNameFlow::PathNode source,
+  SensitiveCookieNameFlow::PathNode sink
+where
+  isSensitiveCookie(cw, name, source, sink) and


name isn't used in the select clause, and this predicate isn't called anywhere else, so you can remove that argument from the predicate. And then you can delete isSource(DataFlow::Node source, string val) and just put all the logic in `isSource(DataFlow::Node source).

owen-mc · 2025-11-18T16:25:13Z

go/ql/test/query-tests/Security/CWE-1004/vendor/modules.txt

Remove mentions of gorilla from this file.

owen-mc · 2025-11-18T16:25:22Z

go/ql/test/query-tests/Security/CWE-1004/go.mod

Remove mentions of gorilla from this file.

owen-mc · 2025-11-18T16:25:33Z

go/ql/test/query-tests/Security/CWE-614/vendor/github.com/gorilla/sessions/stub.go

Delete this file.

owen-mc · 2025-11-18T16:25:50Z

go/ql/test/query-tests/Security/CWE-614/vendor/modules.txt

+# github.com/gorilla/sessions v1.2.1
+## explicit
+github.com/gorilla/sessions


Suggested change

# github.com/gorilla/sessions v1.2.1

## explicit

github.com/gorilla/sessions

owen-mc · 2025-11-18T16:26:11Z

go/ql/test/query-tests/Security/CWE-614/go.mod

+
+require (
+    github.com/gin-gonic/gin v1.7.1
+    github.com/gorilla/sessions v1.2.1


Suggested change

github.com/gorilla/sessions v1.2.1

owen-mc · 2025-11-18T16:44:00Z

Have you checked how autofix works on these queries?

github-actions bot added the Go label Nov 5, 2025

github-advanced-security bot found potential problems Nov 5, 2025

View reviewed changes

github-actions bot added the documentation label Nov 9, 2025

joefarebrother force-pushed the go-insecure-cookie branch 2 times, most recently from d00f670 to 2805a60 Compare November 10, 2025 09:31

joefarebrother added 9 commits November 10, 2025 09:48

Implement cookie write concepts and httponly query

79f1818

Fixes, add secure query

5c1db90

Add modeling for gin

9ce5975

Add tests

533b633

Fixes and doc updates

c7a8712

Add qhelp & fix tests

87bf748

Remove experimental query

c4e9b10

Update integration tests

1c96aff

Add change note

999eff1

joefarebrother force-pushed the go-insecure-cookie branch from 6f26b83 to 999eff1 Compare November 10, 2025 09:50

joefarebrother added 2 commits November 10, 2025 10:18

Update integration test

4c18577

Update formatting

837aba8

joefarebrother marked this pull request as ready for review November 10, 2025 10:35

joefarebrother requested a review from a team as a code owner November 10, 2025 10:35

Copilot AI review requested due to automatic review settings November 10, 2025 10:35

Copilot AI reviewed Nov 10, 2025

View reviewed changes

Fix typos

a078a4e

owen-mc requested changes Nov 11, 2025

View reviewed changes

joefarebrother added 3 commits November 17, 2025 15:07

Address reviews - rename and update doc comments

7decc2b

Refactor parts of SensitiveCookieNameConfig

e2f0fa8

Split SecureCookies into query specific files

ca68c89

owen-mc reviewed Nov 18, 2025

View reviewed changes

	* A write to an HTTP cookie object.
	* A write to an option of an HTTP cookie object.

	* extend `HTTP::CookieOptions` instead.
	* extend `HTTP::CookieOptionWrite` instead.

	* extend `HTTP::CookieOptions::Range` instead.
	* extend `HTTP::CookieOptionWrite::Range` instead.

	# github.com/gorilla/sessions v1.2.1
	## explicit
	github.com/gorilla/sessions

Go: Promote non-httponly cookie query, and add insecure cookie query #20762

Are you sure you want to change the base?

Go: Promote non-httponly cookie query, and add insecure cookie query #20762

Conversation

joefarebrother commented Nov 5, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Cookie 'HttpOnly' attribute is not set to true

Recommendation

Example

References

Cookie 'Secure' attribute is not set to true

Recommendation

Example

References

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

owen-mc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

joefarebrother commented Nov 17, 2025

Uh oh!

owen-mc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

owen-mc commented Nov 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

github-actions bot commented Nov 10, 2025 •

edited

Loading